Skip to content

Add add-crd-gen line to Makefile, regenerate console extension CRDs#101

Closed
benjaminapetersen wants to merge 3 commits intoopenshift:masterfrom
benjaminapetersen:makefile/update
Closed

Add add-crd-gen line to Makefile, regenerate console extension CRDs#101
benjaminapetersen wants to merge 3 commits intoopenshift:masterfrom
benjaminapetersen:makefile/update

Conversation

@benjaminapetersen
Copy link
Copy Markdown
Contributor

@benjaminapetersen benjaminapetersen commented Oct 14, 2019 •
edited
Loading

Generating new code for CRDs from openshift/api doesn't seem to be working. Working on a fix.

  • Adds a line to Makefile : $(call add-crd-gen,manifests,$(CRD_SCHEMA_GEN_APIS),./manifests,./manifests).
  • Adds vendor after make update-deps as separate commit.
  • Regenerates console extension CRDs as 3rd commit, which happens to also pickup apiserver and infrastructure changes.

/cc @spadgett @bparees
/assign @damemi

@benjaminapetersen benjaminapetersen changed the title Add add-crd-gen line to Makefile [WIP] Add add-crd-gen line to Makefile Oct 14, 2019
@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 14, 2019
@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 14, 2019
@benjaminapetersen benjaminapetersen mentioned this pull request Oct 14, 2019
Merged
1 task
@benjaminapetersen benjaminapetersen changed the title [WIP] Add add-crd-gen line to Makefile Add add-crd-gen line to Makefile Oct 14, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 14, 2019
@damemi
Copy link
Copy Markdown
Contributor

damemi commented Oct 14, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 14, 2019
@openshift-ci-robot
Copy link
Copy Markdown

openshift-ci-robot commented Oct 14, 2019

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: benjaminapetersen, damemi
To complete the pull request process, please assign mfojtik
You can assign the PR to them by writing /assign @mfojtik in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@benjaminapetersen benjaminapetersen changed the title Add add-crd-gen line to Makefile Add add-crd-gen line to Makefile, regenerate console extension CRDs Oct 14, 2019
@benjaminapetersen
Copy link
Copy Markdown
Contributor Author

benjaminapetersen commented Oct 14, 2019

/cc deads2k

@spadgett
Copy link
Copy Markdown
Member

spadgett commented Oct 14, 2019

Do we want to wait for openshift/api#465 ?

@benjaminapetersen
Copy link
Copy Markdown
Contributor Author

benjaminapetersen commented Oct 15, 2019

That or the next regen will pick it up. Since this PR contains a fix in the Makefile I'm tempted to get it merged and do openshift/api#465 as a follow.

@openshift-ci-robot
Copy link
Copy Markdown

openshift-ci-robot commented Oct 15, 2019

New changes are detected. LGTM label has been removed.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Oct 15, 2019
@benjaminapetersen
Copy link
Copy Markdown
Contributor Author

benjaminapetersen commented Oct 15, 2019

Regenerated to pick up 1 new file, looks like a lot came with it.

@benjaminapetersen
Copy link
Copy Markdown
Contributor Author

benjaminapetersen commented Oct 15, 2019

/retest

Timeouts.

soltysh
soltysh suggested changes Oct 16, 2019
View reviewed changes
Comment thread Makefile
CRD_SCHEMA_GEN_APIS := $(shell echo ./vendor/github.com/openshift/api/{authorization/v1,config/v1,quota/v1,security/v1,operator/v1alpha1,console/v1})
CRD_SCHEMA_GEN_VERSION :=v0.2.1

$(call add-crd-gen,manifests,$(CRD_SCHEMA_GEN_APIS),./manifests,./manifests)
Copy link
Copy Markdown
Contributor

@soltysh soltysh Oct 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be needed. With openshift/api#470 in place and it looks like your bump already has that you should re-use the manifests provided by openshift/api. Sync with @damemi I thought he wanted to do it 😉

Copy link
Copy Markdown
Contributor

@damemi damemi Oct 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Until #102 merges those CRDs in openshift/api aren't picked up anywhere, meaning we still need to update here if you want changes pulled in

@damemi
Copy link
Copy Markdown
Contributor

damemi commented Oct 16, 2019

/hold
We're moving these CRDs to be consumed by this operator from openshift/api right now, and don't want to add more to the list until the current ones are already sorted out. See #102

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 16, 2019
@openshift-bot
Copy link
Copy Markdown
Contributor

openshift-bot commented Sep 19, 2020

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci-robot openshift-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 19, 2020
@spadgett
Copy link
Copy Markdown
Member

spadgett commented Sep 21, 2020

/close

@openshift-ci-robot
Copy link
Copy Markdown

openshift-ci-robot commented Sep 21, 2020

@spadgett: Closed this PR.

Details

In response to this:

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sttts sttts Awaiting requested review from sttts

@deads2k deads2k Awaiting requested review from deads2k

2 more reviewers

@damemi damemi damemi left review comments

@soltysh soltysh soltysh requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

@damemi damemi

Labels

do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@benjaminapetersen @damemi @openshift-ci-robot @spadgett @openshift-bot @soltysh